Skip to content

Revert "[PM-29567] Pin binary cargo tools via cargo-run-bin"#1164

Merged
coroiu merged 1 commit into
mainfrom
revert-1143-PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file
Jun 3, 2026
Merged

Revert "[PM-29567] Pin binary cargo tools via cargo-run-bin"#1164
coroiu merged 1 commit into
mainfrom
revert-1143-PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file

Conversation

@coroiu

@coroiu coroiu commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Reverts #1143

@coroiu coroiu requested a review from a team as a code owner June 3, 2026 13:08
@coroiu coroiu requested a review from dereknance June 3, 2026 13:08
@coroiu coroiu enabled auto-merge (squash) June 3, 2026 13:12
@djsmith85 djsmith85 removed the request for review from dereknance June 3, 2026 13:12
@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

🔍 SDK Breaking Change Detection

SDK Version: revert-1143-PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file (e947e8d)

⚠️ If breaking changes are detected, a corresponding pull request addressing them must be ready for merge in the affected client repository.

Client Status Details
typescript ✅ No breaking changes detected Compilation passed with new SDK version - View Details
android ✅ No breaking changes detected Compilation passed with new SDK version - View Details

Breaking change detection uses the build of the SDK from this branch, including any incompatibities pre-existing on or merged into this branch. Check the workflow logs to confirm.
Results update as workflows complete.

@coroiu coroiu merged commit d2118d2 into main Jun 3, 2026
69 checks passed
@coroiu coroiu deleted the revert-1143-PM-29567-move-all-binary-cargo-utilities-into-a-separate-shell-file branch June 3, 2026 13:27
bw-ghapp Bot added a commit to bitwarden/sdk-swift that referenced this pull request Jun 3, 2026
@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.12%. Comparing base (a2734ac) to head (e947e8d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1164   +/-   ##
=======================================
  Coverage   84.12%   84.12%           
=======================================
  Files         446      446           
  Lines       58817    58817           
=======================================
  Hits        49478    49478           
  Misses       9339     9339           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coroiu added a commit that referenced this pull request Jun 9, 2026
)

## 🎟️ Tracking

- https://bitwarden.atlassian.net/browse/PM-38720
- https://bitwarden.atlassian.net/browse/VULN-613 (dependency approval)
- https://bitwarden.atlassian.net/browse/PM-29567 (original ticket)

## 📔 Objective

Re-introduce the `cargo-run-bin` pinning that was merged in #1143 and
reverted in #1164 pending AppSec dependency approval.

`cargo-run-bin` is now approved (VULN-613) with one explicit condition:
do not configure the `cargo-binstall` integration. This PR re-applies
the original work and adds the binstall guard that condition requires.

## What's in this PR

1. **The original cargo-run-bin work** (cherry-picked from #1143 + its
three follow-up fixes that got the lint matrix green):
- Pin every binary cargo tool used by CI and `scripts/lint.sh` in one
place (root `Cargo.toml` `[workspace.metadata.bin]`), invoked via `cargo
bin <tool>`.
- Pass clippy `-D warnings` on the command line rather than via the
step's `RUSTFLAGS` (avoids leaking into the from-source builds of
`clippy-sarif`/`sarif-fmt`).
- dylint: invoke `cargo-dylint` directly with the real `dylint-link` on
an absolute PATH (bypassing cargo-run-bin's PATH shim, which fails from
`support/lints`); build only the two tools dylint needs.
- Dedicated `actions/cache` for `.bin`, keyed per check on `Cargo.toml`
+ `rust-toolchain.toml`.

2. **New: binstall guard** (`scripts/check-no-binstall.sh`):
- cargo-run-bin uses `cargo-binstall` when EITHER a `binstall` alias is
set in `.cargo/config.toml`, OR `cargo-binstall` is on PATH — pulling
pre-built binaries from third-party mirrors (QuickInstall) instead of
building from auditable crates.io sources.
- Fails the build if the `binstall` alias is configured (always blocking
— this is what we control in the repo).
- In CI (`GITHUB_ACTIONS=true`), fails if `cargo-binstall` is on PATH.
Locally, prints a warning instead — we can't police individual dev
machines, but devs are told.
- Wired into `scripts/lint.sh` `require_cargo_bin` and added as an
explicit step after `Install cargo-run-bin` in every workflow that uses
`cargo bin`.

## 🚨 Breaking Changes

None for product code. CI/dev tooling only.

## Test plan

- [ ] All lint matrix checks pass (clippy, dylint, sort, udeps, ...).
- [ ] `build-android`, `build-rust-crates`, `check-powerset`,
`rust-test`, `version-bump` workflows still pass.
- [ ] `npm run lint` works locally with `cargo-run-bin` installed.
- [ ] The new "Check cargo-binstall is not configured" step appears (and
passes) in every affected workflow.
- [ ] (Manual) Adding `binstall = "binstall"` under `[alias]` in
`.cargo/config.toml` makes the script (and CI) fail.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants